Simplify fill! bootstrapping and remove @inbounds for AbstractArray
#59181
+41
−49
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oscardssmith
approved these changes
Aug 1, 2025
|
imo for arbitrary abstractarray the compiler should be able to figure out the inbounds itself (but I acknowledge that in practice that often won't happen) |
|
I'm also hesitant to start ripping out abstract array julia> function myfill!(A::AbstractArray{T}, x) where T
xT = convert(T, x)
for I in eachindex(A)
A[I] = xT
end
A
end
myfill! (generic function with 1 method)
julia> @btime fill!(reshape(view($(Array{Int}(undef, 10000,2)), 1:2:10000,1:2), 2500,4), 1);
5.465 μs (0 allocations: 0 bytes)
julia> @btime myfill!(reshape(view($(Array{Int}(undef, 10000,2)), 1:2:10000,1:2), 2500,4), 1);
5.472 μs (0 allocations: 0 bytes) |
|
SparseMatrix would be my guess. |
Good news, this boundscheck is the least of your performance woes when you |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Notably, the compiler will not be able to elide bounds checking for all arrays so this could be a performance hit in some cases (otoh better effects could be a performance boon).
I think that
is one of the safest possible uses of
@inboundson an abstract array and any resulting UB is the fault of the array type so I'm not actually enthusiastic about this change. I made the change based on #59174 (comment) and #59174 (comment) but am now having second thoughts.cc. @giordano @oscardssmith